perf(webapp,run-store): grouped run-ops reads + mint-kind flip grace#4227
perf(webapp,run-store): grouped run-ops reads + mint-kind flip grace#4227d-cs wants to merge 22 commits into
Conversation
Several run-ops read paths issued one database query per item where a single batched query returns the same data: batch results hydrated each member run separately, the run retrieve endpoint resolved each locked worker version separately, the dedicated-schema relation hydrator ran per row, and a few services read runs in per-id loops. These now group into batched reads. Adds a grouped findRunsByIds to the run store (one residency-partitioned read for a set of run ids) and routes the presenters and services through it. The dedicated-schema relation hydrator batches across all rows per relation. The waitpoint connected-runs read also regains its display limit at the fetch, so it no longer loads every connected run into memory before slicing to five.
…n unrequested findRunsByIds forces `id` into the select so it can key the result map, but did not remove it afterwards, so returned rows carried an `id` the declared payload type did not include. Delete the injected id from each value when the caller's select did not request it; the map stays keyed by id.
…list The waitpoint detail view capped the connected-run lookup before checking the runs still existed, so a run deleted after connecting could take a slot ahead of a live one and under-count (or empty) the list. Join to the run table so the cap only ever lands on connections whose run still exists.
Batched relation hydration returned the same row object to every parent linking the same target, so two parents connected to one run shared a single instance and an in-place edit to one leaked into the other. Return a shallow clone on the bare-projection path so each parent gets a distinct object.
The connected-runs relation read fetched every connection id for a waitpoint with no existence check or limit, so a heavily-connected waitpoint could pull an unbounded id list into the grouped run lookup. Existence-join to the run table and cap at the shared connected-runs limit on both schema branches, and slice the cross-store union to the same bound.
The dedicated-schema relation hydrators fetched full target rows and then stripped unrequested fields in memory, pulling wide columns even when the caller only selected a couple. Push the caller's select into the target findMany so only the requested columns are read. No change to results.
Flipping an organization's run-ops mint kind (which database new runs are minted on) took effect independently per process as each cached value expired. For a window after a flip, two concurrent triggers using the same idempotency key could mint on different databases, where the per-database unique constraint can't dedupe them, producing a duplicate run. The flip is now a deterministic wall-clock cutover: the admin feature-flag routes stamp the previously-effective kind and a flip timestamp onto the organization, and the mint read resolves the old kind until flippedAt + grace (default 90s, chosen to outlast the caches a flip drains through), after which every process crosses to the new kind together. During the window all processes agree on one database, so a concurrent same-key collision lands on a single database and the existing unique-constraint retry returns the one idempotent run. A same-target re-save carries the in-flight stamp forward, so it can't slide the cutover.
…clock The grace-window cutover was timestamped with the webapp process's own clock, so clock skew between processes could shift the boundary. Read the time from the control-plane database instead, so the flip is anchored to one authoritative clock rather than whichever process handled the flag change.
The grace cutover compares the reader's wall clock against a DB-clock flippedAt stamp, so it assumes clock-synced hosts (skew well under the grace window). Records that invariant and the accepted residual on effectiveMintKind. No behavior change.
Global runOpsMintKind flips now stamp the previous kind and flip time like per-org flips, so a global flip is a deterministic cutover and cannot route two concurrent triggers that share an idempotency key to different databases. The resolver reads the grace stamp from the same source as the kind.
…isses RoutingRunStore.findRun routed a classifiable id to its single owning store by id shape and returned null on a miss, so a run whose physical residency diverges from its id shape would 404 even though it exists. Read the owning store first, then fall back to the other store only on a miss, mirroring the unrouted fan-out. The found-in-owning-store path is unchanged (single read).
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout. (27)
🧰 Additional context used📓 Path-based instructions (10)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/app/routes/**/*.ts📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
🧠 Learnings (20)📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-06-13T19:53:13.759ZApplied to files:
📚 Learning: 2026-06-17T17:13:49.929ZApplied to files:
📚 Learning: 2026-06-23T13:04:21.413ZApplied to files:
📚 Learning: 2026-05-07T12:25:18.271ZApplied to files:
📚 Learning: 2026-05-28T20:02:10.647ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
📚 Learning: 2026-06-25T18:21:51.905ZApplied to files:
📚 Learning: 2026-07-03T17:10:21.498ZApplied to files:
📚 Learning: 2026-05-18T14:40:02.173ZApplied to files:
📚 Learning: 2026-06-04T18:16:35.386ZApplied to files:
📚 Learning: 2026-06-09T17:58:04.699ZApplied to files:
📚 Learning: 2026-06-16T09:19:47.637ZApplied to files:
📚 Learning: 2026-03-29T19:16:28.864ZApplied to files:
📚 Learning: 2026-06-09T16:27:26.195ZApplied to files:
📚 Learning: 2026-05-05T09:38:02.512ZApplied to files:
📚 Learning: 2026-05-14T08:21:07.614ZApplied to files:
🔇 Additional comments (5)
WalkthroughAdds deterministic mint-kind flip grace handling with server-side timestamp stamping, new feature-flag metadata, and cache-aware resolution. Replaces several per-record database reads with grouped queries and batched relation hydration across legacy and dedicated stores. Adds bounded connected-run retrieval, residency fallback routing, projection isolation, and presenter integrations. Expands integration coverage for grouped reads, split-store behavior, schedule residency, connected-run limits, mint flips, and cross-database waitpoint resumption. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…int kind stampMintKindFlip injected the default mint kind into every org feature-flag save, so an unrelated flag change wrote an explicit per-org runOpsMintKind override, pinning the org to that kind and making a later global flip silently skip it. Only stamp when the save actually sets runOpsMintKind.
The raw queries reading a waitpoint's connected runs relied on search_path, so they broke on deployments using a non-public database schema. Qualify the tables with the configured schema so they resolve correctly.
…ields An org's first per-org mint-flag override computed its cutover grace window against the wrong baseline, which could skip the grace window and briefly let two concurrent triggers sharing an idempotency key resolve to different databases. Seed the baseline from the effective global flag, strip the derived stamp fields from the request body, and apply the read-and-stamp atomically.
…d schema The dedicated-schema hydrator for a waitpoint's connectedRuns relation fetched an unbounded list of connections and full run rows. Bound it per parent with a window function so a waitpoint with many connections cannot pull a large result set for a display-only field.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…w SQL Replaces the raw connected-runs query, which JOINed the waitpoint connection table onto the large TaskRun table, with two indexed ORM reads joined in memory: read the connection rows, then resolve the runs by id. The id lookup can only plan as a primary-key lookup, so the query planner never scans TaskRun, and Prisma qualifies the schema per client so the previous manual schema handling is gone.
How idempotency was testedTwo properties were checked directly against the running system, one for each failure mode a two-database split introduces: 1. Concurrent dedup, on each database. With the residency flag fixed to one database, a burst of concurrent triggers sharing a single idempotency key was issued. The result is exactly one run: the first insert wins and every other caller receives that same run id, because the per-database unique index on the idempotency key rejects the duplicate inserts. Repeated with the flag fixed to the other database: same result, one run. 2. Dedup across the residency flip. A key is first used while new runs mint into database A. The flag is then flipped so new runs mint into database B, and the same key is triggered again. It returns the original run id from database A rather than minting a second run in database B. This is the case a per-database unique constraint cannot catch on its own (the constraint in B cannot see the row in A), so it confirms the pre-mint existing-run lookup spans both databases. Together these close both gaps a split could open: a race within one database (rejected by the unique constraint) and a key reused across the cutover (caught by the cross-database lookup before a second run is minted). |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The global feature-flag route read, stamped, and wrote the mint-kind grace metadata without a lock, so two concurrent global flips could clobber each other's grace stamp and skip the deterministic cutover window (briefly reopening the cross-database duplicate window the grace exists to close). Wrap the read/stamp/write in one transaction under an advisory lock, mirroring the per-org routes. An advisory lock rather than row FOR UPDATE because the global flag rows may not exist yet on a first flip.
SQL structure and index usage: confirmed goodEvery DB query this PR adds or changes was inventoried and checked with Connected-runs in the presenter (now ORM, no raw SQL). Two indexed reads joined in memory, so the runs table is only ever a primary-key lookup: SELECT "id", "taskRunId" FROM "WaitpointRunConnection"
WHERE "waitpointId" = $1 ORDER BY "id" LIMIT 25;
-- Index Scan using WaitpointRunConnection_waitpointId_idx
SELECT "id", "friendlyId" FROM "TaskRun"
WHERE "id" IN ($1, $2, ...) ORDER BY "id" LIMIT 5;
-- Index Scan using TaskRun_pkeyOn the control-plane schema the implicit many-to-many resolves as three primary-key / Grouped relation hydration (run-store). Per-parent (N+1) hydration is collapsed into one Bounded connected-runs (run-store, parameterized raw). The two previously-unbounded paths are now capped, and both plan index-driven: -- dedicated (run-ops): cap per waitpoint via a window
SELECT "taskRunId" FROM (
SELECT c."taskRunId",
ROW_NUMBER() OVER (PARTITION BY c."waitpointId" ORDER BY c."id") AS rn
FROM "WaitpointRunConnection" c
JOIN "TaskRun" t ON t."id" = c."taskRunId"
WHERE c."waitpointId" = ANY($1)
) ranked WHERE rn <= 5;
-- Index Scan WaitpointRunConnection_waitpointId_idx + Index Only Scan TaskRun_pkey
-- legacy (control-plane)
SELECT c."A" FROM "_WaitpointRunConnections" c
JOIN "TaskRun" t ON t."id" = c."A" WHERE c."B" = $1 LIMIT 5;
-- Index Scan _WaitpointRunConnections_B_index + Index Only Scan TaskRun_pkeyBatch and flag reads. Batch members resolve by Honest notes, none of which are scan or index risks:
|
2 participants
Summary
Two threads on the run-ops split path.
Read path: per-item run reads are batched into grouped queries, a waitpoint's connected-run reads are bounded, and the dedicated-schema relation hydrators fetch only the requested columns instead of whole rows. Retrieve also falls back to the other database when a routed read misses, so a run whose physical residency diverges from its id shape is still found rather than returning a spurious not-found. Fewer and lighter queries on the run read path, with no change to results.
Mint-kind flip safety: flipping which database new runs mint to is now a deterministic wall-clock cutover, for both per-org and global flips. For a grace window every process resolves the same database, so a flip cannot route two concurrent triggers that share an idempotency key to different databases (which would bypass the per-database unique constraint and create a duplicate run).
Supersedes the earlier #4205 and #4208.
Draft: validation in progress.